Add switch test #526
Add switch test #526
Conversation
|
|
||
| result.Document("Instrumentation.SelectionStatements.cs") | ||
| .AssertLinesCovered(BuildConfiguration.Release, (24, 1), (26, 0), (28, 0)) | ||
| .AssertBranchesCovered(BuildConfiguration.Release, (24, 1, 1)) |
There was a problem hiding this comment.
This looks like it make the test very dependent on the optimisations in the compiler, so they could start breaking on a different SDK version. Since these tests are about matching specific lines it seems better to just use Debug runs and not include any expected outcome for Release runs.
There was a problem hiding this comment.
This looks like it make the test very dependent on the optimisations in the compiler,
Yep it's true...talked also with Andrew #396 (comment) and we had same issue in past #389
Yes we could test only for debug config...but why if some bug happen on release and not on debug config for some reason related to "instrumentation" bug?If in future we put some conditional code related to some new roslyn behaviour?Maybe I'm too concern about something that won't never happen 😅 , but could be a good "doorbell", and maybe we could add the possibility to run test only for specific SDK like a custom xunit attribute [RunOnSdks("2.2.301","..")], to be very rigorous.
Another thing that I'm not sure is if also "debug" config will be stable...I mean also that config could depend on different SDK version, less likely but I'm not 100% sure.
BTW I don't have a super strong opinion and we could test only debug config...I tried to be complete as possible...
Thank's for review Peter!!!
@tonerdo thoughts?
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
I'll merge this, low risk I need code for next PRs. |
2 participants
Improve "real" instrumentation testing
cc: @tonerdo @petli